Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fenwick Tree Updates #4803

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

TheGamingMousse
Copy link
Contributor

Place an "x" in the corresponding checkbox if it is done or does not apply to this pull request.

  • I have tested my code.

  • I have added my solution according to the steps here.

  • I have followed the code conventions mentioned here.

    • I understand that if it is clear that I have not attempted to follow these conventions, my PR will be closed.
    • If changes are requested, I will re-request a review after addressing them.
  • I have linked this PR to any issues that it closes.

  • added a (in my opinion) cleaner 2d bit impl (which is also more in line w/ the PURS bit impl)

  • added a slightly different verison of the offline bit impl

    • like the bit impl in the purs module, everything is zero indexed here
    • greatly improved the efficiency of the code by passing in the vector for the ind function as a reference

Copy link
Contributor

@SansPapyrus683 SansPapyrus683 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my goat...

content/5_Plat/2DRQ.mdx Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
TheGamingMousse and others added 2 commits September 28, 2024 11:05
Co-authored-by: Kevin Sheng <55369003+SansPapyrus683@users.noreply.github.com>
Co-authored-by: Kevin Sheng <55369003+SansPapyrus683@users.noreply.github.com>
class BIT2D {
/**
* Offline 2D Fenwick Tree implementation.
* Note that n needs to be of a reasonable size, and
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does reasonable size mean

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe i should just specify that the rows aren't coordinate compressed in the template itself

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps smth smth "memory takes up is O(n^2) actually haha"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no it doesnt take O(n^2) memory tho lmao, which is why its actually kinda useful

its like O(n log n) memory or similar, depending on the # of update queries

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well i guess n=1e5 would work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah n=1e5 won’t tle

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i mean then i'm not sure why this warning is necessary
when does this start breaking down

Copy link
Contributor Author

@TheGamingMousse TheGamingMousse Sep 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd guess n=1e6 or n=1e7

the main reason is because the # of columns can scale to a stupid big number (cuz we are coordinate compressing), but we aren't coordinate compressing the rows here for the sake of making the implementation easier

content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
content/5_Plat/2DRQ.mdx Outdated Show resolved Hide resolved
TheGamingMousse and others added 2 commits September 28, 2024 11:28
Co-authored-by: Kevin Sheng <55369003+SansPapyrus683@users.noreply.github.com>
Co-authored-by: Kevin Sheng <55369003+SansPapyrus683@users.noreply.github.com>
content/5_Plat/2DRQ.mdx Show resolved Hide resolved
@@ -418,7 +441,7 @@ int main() {

<Warning title="Implementation Note">

As mentioned earlier, the above `BIT2D` implementation is significantly slower than Benq's `OffBIT2D` and, in fact, will get TLE on the Soriya's Programming Project; this is due to the large amount of calls to `vector.resize` it makes.
As mentioned earlier, the above `OfflineBIT2D` implementation is significantly slower than Benq's `OffBIT2D` and, in fact, will get TLE on the Soriya's Programming Project.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think you can still explain why it's slower

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok the thing is the previous explanation for why it got constant factored is wrong tho (or maybe I am underestimating the space complexity for this problem) because it doesn't call .resize all that many times

so I'm p sure it just gets constant factored out in this case

idk if benq's passes, but if it does it's because he does some cursed stuff and actually flattens out the binary indexed trees into just one really big binary indexed tree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants